Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Move remove_if_else pass after second inlining #4976

Merged
merged 3 commits into from
May 3, 2024
Merged

fix: Move remove_if_else pass after second inlining #4976

merged 3 commits into from
May 3, 2024

Conversation

vezenovm
Copy link
Contributor

@vezenovm vezenovm commented May 3, 2024

Description

Problem*

Resolves

Summary*

For the test added in this PR I bumped the max_iter in try_merge_only_changed_indices. We then get the following panic:

The application panicked (crashed).
Message:  internal error: entered unreachable code: All Value::Instructions should already be known during inlining after creating the original inlined instruction. Unknown value v41 = Instruction { instruction: Id(76), position: 0, typ: Array([Numeric(Unsigned { bit_size: 32 })], 30) }
Location: compiler/noirc_evaluator/src/ssa/opt/inlining.rs:253

We have to inline immediately following mem2reg before extra instructions are added by the remove_if_else pass.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 3, 2024 •
edited
Loading

Changes to circuit sizes

Generated at commit: 9e3d1aa53374f01e7d57db5ea2f2a4f6e13db3a4, compared to commit: a87c655c6c8c077c71e3372cc9181b7870348a3d

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
fold_numeric_generic_poseidon -4 ✅ -28.57% -4 ✅ -21.05%
fold_call_witness_condition -4 ✅ -36.36% -4 ✅ -23.53%
Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression_mem_op_predicate 56 (+12) +27.27% 3,559 (+23) +0.65%
conditional_1 6,040 (-1,131) -15.77% 38,799 (0) 0.00%
no_predicates_numeric_generic_poseidon 55 (-4) -6.78% 10,758 (-4) -0.04%
slice_dynamic_index 1,353 (-158) -10.46% 6,947 (-297) -4.10%
slices 350 (-110) -23.91% 3,420 (-198) -5.47%
hashmap 176,062 (-1,608) -0.91% 367,934 (-26,986) -6.83%
fold_numeric_generic_poseidon 10 (-4) -28.57% 15 (-4) -21.05%
fold_call_witness_condition 7 (-4) -36.36% 13 (-4) -23.53%

@TomAFrench TomAFrench added this pull request to the merge queue May 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 3, 2024
@vezenovm vezenovm added this pull request to the merge queue May 3, 2024
Merged via the queue into master with commit 96fb3e9 May 3, 2024
43 checks passed
@vezenovm vezenovm deleted the mv/remove-if-else-reorder branch May 3, 2024 21:48
@kevaundray kevaundray mentioned this pull request May 3, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request May 6, 2024
@kevaundray
chore: Release Noir(0.29.0) (#4905)
2da2899 
🤖 I have created a release *beep* *boop*
---


<details><summary>0.29.0</summary>

## [0.29.0](v0.28.0...v0.29.0)
(2024-05-03)


### ⚠ BREAKING CHANGES

* use `distinct` return value witnesses by default
([#4951](#4951))
* Bit shift is restricted to u8 right operand
([#4907](#4907))

### Features

* Add `#[inline(tag)]` attribute and codegen
([#4913](#4913))
([1ec9cdc](1ec9cdc))
* Add de-sugaring for `impl Trait` in function parameters
([#4919](#4919))
([8aad2e4](8aad2e4))
* Add variable size sha256
([#4920](#4920))
([dbfca58](dbfca58))
* Bit shift is restricted to u8 right operand
([#4907](#4907))
([c4b0369](c4b0369))
* Complex outputs from acir call
([#4952](#4952))
([2e085b9](2e085b9))
* **experimental:** `comptime` globals
([#4918](#4918))
([8a3c7f1](8a3c7f1))
* Handle `BrilligCall` opcodes in the debugger
([#4897](#4897))
([b380dc4](b380dc4))
* Handle `no_predicates` attribute
([#4942](#4942))
([0ce04d3](0ce04d3))
* Handle empty response foreign calls without an external resolver
([#4959](#4959))
([0154bde](0154bde))
* Optimize array sets in if conditions (alternate version)
([#4716](#4716))
([a87c655](a87c655))
* Use `distinct` return value witnesses by default
([#4951](#4951))
([5f1b584](5f1b584))


### Bug Fixes

* Ban self-referential structs
([#4883](#4883))
([800f670](800f670))
* Discard ref counts during unrolling
([#4923](#4923))
([91062db](91062db))
* Ensure where clauses propagated to trait default definitions
([#4894](#4894))
([aaac0f6](aaac0f6))
* Move remove_if_else pass after second inlining
([#4976](#4976))
([96fb3e9](96fb3e9))
* Nested array equality
([#4903](#4903))
([0cf2e2a](0cf2e2a))
* Require for all foldable functions to use distinct return
([#4949](#4949))
([d4c6806](d4c6806))
* Use annotated type when checking declaration
([#4966](#4966))
([f7fa696](f7fa696))
</details>

<details><summary>0.45.0</summary>

## [0.45.0](v0.44.0...v0.45.0)
(2024-05-03)


### ⚠ BREAKING CHANGES

* Bit shift is restricted to u8 right operand
([#4907](#4907))
* contract interfaces and better function calls
(AztecProtocol/aztec-packages#5687)
* change backend width to 4
(AztecProtocol/aztec-packages#5374)
* Use fixed size arrays in black box functions where sizes are known
(AztecProtocol/aztec-packages#5620)
* trap with revert data
(AztecProtocol/aztec-packages#5732)
* **acir:** BrilligCall opcode
(AztecProtocol/aztec-packages#5709)
* remove fixed-length keccak256
(AztecProtocol/aztec-packages#5617)
* storage_layout and `#[aztec(storage)]`
(AztecProtocol/aztec-packages#5387)
* **acir:** Add predicate to call opcode
(AztecProtocol/aztec-packages#5616)
* contract_abi-exports
(AztecProtocol/aztec-packages#5386)
* Brillig typed memory
(AztecProtocol/aztec-packages#5395)
* **acir:** Program and witness stack structure
(AztecProtocol/aztec-packages#5149)
* automatic NoteInterface and NoteGetterOptions auto select
(AztecProtocol/aztec-packages#4508)
* Acir call opcode
(AztecProtocol/aztec-packages#4773)
* Support contracts with no constructor
(AztecProtocol/aztec-packages#5175)
* Internal as a macro
(AztecProtocol/aztec-packages#4898)
* move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
* note type ids
(AztecProtocol/aztec-packages#4500)
* rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
* init storage macro
(AztecProtocol/aztec-packages#4200)
* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
* Sync commits from `aztec-packages`
([#4144](#4144))
* Breaking changes from aztec-packages
([#3955](#3955))

### Features

* Acir call opcode
(AztecProtocol/aztec-packages#4773)
([c3c9e19](c3c9e19))
* **acir_gen:** Brillig stdlib
([#4848](#4848))
([0c8175c](0c8175c))
* **acir_gen:** Fold attribute at compile-time and initial non inlined
ACIR (AztecProtocol/aztec-packages#5341)
([a0f7474](a0f7474))
* **acir:** Add predicate to call opcode
(AztecProtocol/aztec-packages#5616)
([2bd006a](2bd006a))
* **acir:** BrilligCall opcode
(AztecProtocol/aztec-packages#5709)
([0f9ae0a](0f9ae0a))
* **acir:** Program and witness stack structure
(AztecProtocol/aztec-packages#5149)
([13eb71b](13eb71b))
* **acvm_js:** Execute program
([#4694](#4694))
([386f6d0](386f6d0))
* **acvm:** Execute multiple circuits
(AztecProtocol/aztec-packages#5380)
([a0f7474](a0f7474))
* Add bit size to const opcode
(AztecProtocol/aztec-packages#4385)
([158c8ce](158c8ce))
* Add CMOV instruction to brillig and brillig gen
(AztecProtocol/aztec-packages#5308)
([13eb71b](13eb71b))
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
([158c8ce](158c8ce))
* Add instrumentation for tracking variables in debugging
([#4122](#4122))
([c58d691](c58d691))
* Add poseidon2 opcode implementation for acvm/brillig, and Noir
([#4398](#4398))
([10e8292](10e8292))
* Add return values to aztec fns
(AztecProtocol/aztec-packages#5389)
([2bd006a](2bd006a))
* Add support for overriding expression width
([#4117](#4117))
([c8026d5](c8026d5))
* Added cast opcode and cast calldata
(AztecProtocol/aztec-packages#4423)
([78ef013](78ef013))
* Allow brillig to read arrays directly from memory
(AztecProtocol/aztec-packages#4460)
([158c8ce](158c8ce))
* Allow nested arrays and vectors in Brillig foreign calls
(AztecProtocol/aztec-packages#4478)
([158c8ce](158c8ce))
* Allow variables and stack trace inspection in the debugger
([#4184](#4184))
([bf263fc](bf263fc))
* Automatic NoteInterface and NoteGetterOptions auto select
(AztecProtocol/aztec-packages#4508)
([13eb71b](13eb71b))
* **avm:** Back in avm context with macro - refactor context
(AztecProtocol/aztec-packages#4438)
([158c8ce](158c8ce))
* **avm:** Brillig CONST of size &gt; u128
(AztecProtocol/aztec-packages#5217)
([c3c9e19](c3c9e19))
* **avm:** Integrate AVM with initializers
(AztecProtocol/aztec-packages#5469)
([2bd006a](2bd006a))
* **aztec-nr:** Initial work for aztec public vm macro
(AztecProtocol/aztec-packages#4400)
([158c8ce](158c8ce))
* Backpropagate constants in ACIR during optimization
([#3926](#3926))
([aad0da0](aad0da0))
* Bit shift is restricted to u8 right operand
([#4907](#4907))
([c4b0369](c4b0369))
* Breaking changes from aztec-packages
([#3955](#3955))
([5be049e](5be049e))
* Brillig heterogeneous memory cells
(AztecProtocol/aztec-packages#5608)
([305bcdc](305bcdc))
* Brillig IR refactor
(AztecProtocol/aztec-packages#5233)
([c3c9e19](c3c9e19))
* Brillig pointer codegen and execution
(AztecProtocol/aztec-packages#5737)
([0f9ae0a](0f9ae0a))
* Brillig typed memory
(AztecProtocol/aztec-packages#5395)
([0bc18c4](0bc18c4))
* Change backend width to 4
(AztecProtocol/aztec-packages#5374)
([0f9ae0a](0f9ae0a))
* Check initializer msg.sender matches deployer from address preimage
(AztecProtocol/aztec-packages#5222)
([c3c9e19](c3c9e19))
* Contract interfaces and better function calls
(AztecProtocol/aztec-packages#5687)
([0f9ae0a](0f9ae0a))
* Contract_abi-exports
(AztecProtocol/aztec-packages#5386)
([2bd006a](2bd006a))
* Evaluation of dynamic assert messages
([#4101](#4101))
([c284e01](c284e01))
* Handle `BrilligCall` opcodes in the debugger
([#4897](#4897))
([b380dc4](b380dc4))
* Impl of missing functionality in new key store
(AztecProtocol/aztec-packages#5750)
([0f9ae0a](0f9ae0a))
* Init storage macro
(AztecProtocol/aztec-packages#4200)
([158c8ce](158c8ce))
* Initial Earthly CI
(AztecProtocol/aztec-packages#5069)
([c3c9e19](c3c9e19))
* Internal as a macro
(AztecProtocol/aztec-packages#4898)
([5f57ebb](5f57ebb))
* **nargo:** Handle call stacks for multiple Acir calls
([#4711](#4711))
([5b23171](5b23171))
* New brillig field operations and refactor of binary operations
(AztecProtocol/aztec-packages#5208)
([c3c9e19](c3c9e19))
* Note type ids
(AztecProtocol/aztec-packages#4500)
([78ef013](78ef013))
* Remove replacement of boolean range opcodes with `AssertZero` opcodes
([#4107](#4107))
([dac0e87](dac0e87))
* Restore hashing args via slice for performance
(AztecProtocol/aztec-packages#5539)
([2bd006a](2bd006a))
* Signed integer division and modulus in brillig gen
(AztecProtocol/aztec-packages#5279)
([c3c9e19](c3c9e19))
* **simulator:** Fetch return values at circuit execution
(AztecProtocol/aztec-packages#5642)
([305bcdc](305bcdc))
* Storage_layout and `#[aztec(storage)]`
(AztecProtocol/aztec-packages#5387)
([2bd006a](2bd006a))
* Support contracts with no constructor
(AztecProtocol/aztec-packages#5175)
([c3c9e19](c3c9e19))
* Sync `aztec-packages`
([#4011](#4011))
([fee2452](fee2452))
* Sync commits from `aztec-packages`
([#4068](#4068))
([7a8f3a3](7a8f3a3))
* Sync commits from `aztec-packages`
([#4144](#4144))
([0205d3b](0205d3b))
* Sync from aztec-packages
([#4483](#4483))
([fe8f277](fe8f277))
* Sync from noir
(AztecProtocol/aztec-packages#5234)
([c3c9e19](c3c9e19))
* Sync from noir
(AztecProtocol/aztec-packages#5286)
([c3c9e19](c3c9e19))
* Sync from noir
(AztecProtocol/aztec-packages#5572)
([2bd006a](2bd006a))
* Sync from noir
(AztecProtocol/aztec-packages#5619)
([2bd006a](2bd006a))
* Sync from noir
(AztecProtocol/aztec-packages#5697)
([305bcdc](305bcdc))
* Sync from noir
(AztecProtocol/aztec-packages#5794)
([0f9ae0a](0f9ae0a))
* Sync from noir
(AztecProtocol/aztec-packages#5814)
([0f9ae0a](0f9ae0a))
* Sync from noir
(AztecProtocol/aztec-packages#5935)
([1b867b1](1b867b1))
* Sync from noir
(AztecProtocol/aztec-packages#5955)
([1b867b1](1b867b1))
* Sync from noir
(AztecProtocol/aztec-packages#5999)
([1b867b1](1b867b1))
* Trap with revert data
(AztecProtocol/aztec-packages#5732)
([0f9ae0a](0f9ae0a))
* Use fixed size arrays in black box functions where sizes are known
(AztecProtocol/aztec-packages#5620)
([0f9ae0a](0f9ae0a))
* Variable length returns
(AztecProtocol/aztec-packages#5633)
([305bcdc](305bcdc))


### Bug Fixes

* **acvm:** Mark outputs of Opcode::Call solvable
([#4708](#4708))
([8fea405](8fea405))
* Avoid huge unrolling in hash_args
(AztecProtocol/aztec-packages#5703)
([305bcdc](305bcdc))
* Catch panics from EC point creation (e.g. the point is at infinity)
([#4790](#4790))
([645dba1](645dba1))
* Don't reuse brillig with slice arguments
(AztecProtocol/aztec-packages#5800)
([0f9ae0a](0f9ae0a))
* Issue 4682 and add solver for unconstrained bigintegers
([#4729](#4729))
([e4d33c1](e4d33c1))
* Noir test incorrect reporting
(AztecProtocol/aztec-packages#4925)
([5f57ebb](5f57ebb))
* Proper field inversion for bigints
([#4802](#4802))
([b46d0e3](b46d0e3))
* Remove panic from `init_log_level` in `acvm_js`
([#4195](#4195))
([2e26530](2e26530))
* Return error rather instead of panicking on invalid circuit
([#3976](#3976))
([67201bf](67201bf))


### Miscellaneous Chores

* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
([158c8ce](158c8ce))
* Move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
([78ef013](78ef013))
* Remove fixed-length keccak256
(AztecProtocol/aztec-packages#5617)
([305bcdc](305bcdc))
* Rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
([158c8ce](158c8ce))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request May 7, 2024
@AztecBot
[1 changes] fix: set index and value to 0 for array_get with predicate (
115665f 
noir-lang/noir#4971)

fix: Compute the correct slice length when coercing from a literal array of complex types (noir-lang/noir#4986)
feat: add `Neg` trait to stdlib (noir-lang/noir#4983)
feat: implement `From` array trait for `BoundedVec` (noir-lang/noir#4927)
chore: Release Noir(0.29.0) (noir-lang/noir#4905)
fix: Move remove_if_else pass after second inlining  (noir-lang/noir#4976)
@AztecBot AztecBot mentioned this pull request May 7, 2024
Merged
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request May 7, 2024
@AztecBot @TomAFrench
feat: Sync from noir (#6234)
02d3d17 
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: set index and value to 0 for array_get with predicate
(noir-lang/noir#4971)
fix: Compute the correct slice length when coercing from a literal array
of complex types (noir-lang/noir#4986)
feat: add `Neg` trait to stdlib
(noir-lang/noir#4983)
feat: implement `From` array trait for `BoundedVec`
(noir-lang/noir#4927)
chore: Release Noir(0.29.0)
(noir-lang/noir#4905)
fix: Move remove_if_else pass after second inlining
(noir-lang/noir#4976)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
This was referenced May 7, 2024
Merged
Merged
TomAFrench added a commit that referenced this pull request May 8, 2024
@TomAFrench
Merge branch 'master' into aztec-packages
d855dff 
* master: (41 commits)
  fix: defer overflow checks for unsigned integers to acir-gen (#4832)
  feat: add support for u16/i16 (#4985)
  chore: split `ops` into `arith` and `bit` modules (#4989)
  chore(ci): run clippy on benchmarks (#4988)
  feat: remove query to backend to get expression width (#4975)
  fix: set index and value to 0 for array_get with predicate (#4971)
  fix: Compute the correct slice length when coercing from a literal array of complex types (#4986)
  feat: add `Neg` trait to stdlib (#4983)
  feat: implement `From` array trait for `BoundedVec` (#4927)
  chore: Release Noir(0.29.0) (#4905)
  fix: Move remove_if_else pass after second inlining  (#4976)
  feat: Optimize array sets in if conditions (alternate version) (#4716)
  chore: rename instruction checks for side effects (#4945)
  chore: Switch Noir JS to use execute program instead of circuit (#4965)
  fix: Use annotated type when checking declaration (#4966)
  feat: handle empty response foreign calls without an external resolver (#4959)
  feat: Complex outputs from acir call (#4952)
  fix: Require for all foldable functions to use distinct return  (#4949)
  feat!: use `distinct` return value witnesses by default (#4951)
  chore(docs): adding matomo tracking (#4898)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAFrench TomAFrench TomAFrench approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vezenovm @TomAFrench